Skip to content

HBASE-30011 Upgrade hbase-server to use junit5 Part1#7947

Open
liuxiaocs7 wants to merge 1 commit intoapache:masterfrom
liuxiaocs7:HBASE-29998
Open

HBASE-30011 Upgrade hbase-server to use junit5 Part1#7947
liuxiaocs7 wants to merge 1 commit intoapache:masterfrom
liuxiaocs7:HBASE-29998

Conversation

@liuxiaocs7
Copy link
Member

@liuxiaocs7 liuxiaocs7 commented Mar 17, 2026

  • Part1 for hbase-server
  • includes filter, namequeues, procedure, procedure2, protobuf, tool, zookeeper
  • see: HBASE-30011 and HBASE-29998

@Apache9
Copy link
Contributor

Apache9 commented Mar 20, 2026

Let's open sub tasks for migrating hbase-server since it is too big? We should change the title of this PR since we haven't upgrade all the tests yet...

@liuxiaocs7
Copy link
Member Author

Let's open sub tasks for migrating hbase-server since it is too big? We should change the title of this PR since we haven't upgrade all the tests yet...

Good suggestion, will open subtasks in Jira!

@liuxiaocs7 liuxiaocs7 changed the title HBASE-29998 Upgrade hbase-server to use junit5 HBASE-30011 Upgrade hbase-server to use junit5 Part1 Mar 20, 2026
@liuxiaocs7 liuxiaocs7 requested a review from Copilot March 20, 2026 09:26
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR is part 1 of migrating hbase-server tests to JUnit 5 (HBASE-30011), updating multiple test suites across filter/namequeues/procedure/protobuf/tool/zookeeper to use JUnit Jupiter APIs and conventions.

Changes:

  • Migrates tests from JUnit 4 (@Test, @Before, @Category, @Rule, @Ignore, expected-exception syntax) to JUnit 5 (@Test, @BeforeEach, @Tag, @Disabled, assertThrows, TestInfo).
  • Refactors bulk-load test structure by introducing shared *TestBase classes and new JUnit 5 test entrypoints (e.g., BulkLoadHFilesTest, BulkLoadHFilesSplitRecoveryTest).
  • Updates parameterized tests to use the project’s JUnit 5 template-based parameterization (@HBaseParameterizedTestTemplate + @TestTemplate + parameters()).

Reviewed changes

Copilot reviewed 68 out of 68 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
hbase-server/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZooKeeperACL.java JUnit 5 migration (Jupiter assertions, lifecycle annotations, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/tool/TestCanaryTool.java JUnit 5 migration; uses TestInfo for method names; updates assertions.
hbase-server/src/test/java/org/apache/hadoop/hbase/tool/SecureBulkLoadHFilesTest.java JUnit 5 migration + refactor to new bulk-load test base.
hbase-server/src/test/java/org/apache/hadoop/hbase/tool/SecureBulkLoadHFilesSplitRecoveryTest.java JUnit 5 migration + secure split-recovery subclass.
hbase-server/src/test/java/org/apache/hadoop/hbase/tool/coprocessor/CoprocessorValidatorTest.java JUnit 5 migration (tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/tool/BulkLoadHFilesTestSFT.java Refactor to new bulk-load base + JUnit 5 migration.
hbase-server/src/test/java/org/apache/hadoop/hbase/tool/BulkLoadHFilesTestBase.java New shared base for bulk-load tests; JUnit 5 updates.
hbase-server/src/test/java/org/apache/hadoop/hbase/tool/BulkLoadHFilesTest.java New JUnit 5 test entrypoint configuring cluster for bulk-load base tests.
hbase-server/src/test/java/org/apache/hadoop/hbase/tool/BulkLoadHFilesSplitRecoveryTestBase.java Refactor to shared base + JUnit 5 updates (assertThrows, TestInfo).
hbase-server/src/test/java/org/apache/hadoop/hbase/tool/BulkLoadHFilesSplitRecoveryTest.java New JUnit 5 test entrypoint configuring cluster for split-recovery base tests.
hbase-server/src/test/java/org/apache/hadoop/hbase/protobuf/TestReplicationProtobuf.java JUnit 5 migration (tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestWALProcedurePrettyPrinter.java JUnit 5 migration (tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestRegionProcedureStoreMigration.java JUnit 5 migration (per-test lifecycle).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestRegionProcedureStore.java JUnit 5 migration (tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/TestHFileProcedurePrettyPrinter.java JUnit 5 migration (tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/RegionProcedureStoreTestProcedure.java JUnit 5 migration (assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure2/store/region/RegionProcedureStoreTestBase.java JUnit 5 migration (BeforeEach/AfterEach).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestZKProcedureControllers.java JUnit 5 migration (BeforeAll/AfterAll, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestZKProcedure.java JUnit 5 migration (BeforeAll/AfterAll, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestProcedureMember.java JUnit 5 migration (AfterEach, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestProcedureManager.java JUnit 5 migration (BeforeAll/AfterAll, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestProcedureDescriber.java JUnit 5 migration (imports, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestProcedureCoordinator.java JUnit 5 migration (AfterEach, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestProcedure.java JUnit 5 migration (BeforeEach, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/TestFailedProcCleanup.java JUnit 5 migration (BeforeAll/AfterEach, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestWALEventTrackerTableAccessor.java JUnit 5 migration (tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestWalEventTrackerQueueService.java JUnit 5 migration; uses TestInfo for names.
hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestWALEventTracker.java JUnit 5 migration (BeforeAll/AfterAll/BeforeEach, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestTooLargeLog.java JUnit 5 migration (BeforeAll/AfterAll, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestSlowLogAccessor.java JUnit 5 migration (BeforeAll/AfterAll/BeforeEach, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/namequeues/TestRpcLogDetails.java JUnit 5 migration (tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestSingleColumnValueFilter.java JUnit 5 migration (assertions, tags, lifecycle).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestSingleColumnValueExcludeFilter.java JUnit 5 migration (assertions, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestSeekHints.java JUnit 5 migration (BeforeAll/AfterAll, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestScanRowPrefix.java JUnit 5 migration; explicit cluster setup via FilterTestingCluster.
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestRegexComparator.java JUnit 5 migration (assertions, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestRandomRowFilter.java JUnit 5 migration (assertions, tags, lifecycle).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestQualifierFilterWithEmptyQualifier.java JUnit 5 migration (assertions, tags, lifecycle).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestPrefixFilter.java JUnit 5 migration (assertions, tags, lifecycle).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestParseFilter.java JUnit 5 migration (assertions, tags, lifecycle).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestPageFilter.java JUnit 5 migration (assertions, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestNullComparator.java JUnit 5 migration (assertions, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestMultiRowRangeFilter.java JUnit 5 migration (assertThrows, tags, TestInfo).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestMultipleColumnPrefixFilter.java JUnit 5 migration (TestInfo, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestInvocationRecordFilter.java JUnit 5 migration (assertions, tags, lifecycle).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestInclusiveStopFilter.java JUnit 5 migration (assertions, tags, lifecycle).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFuzzyRowFilterEndToEndLarge.java JUnit 5 migration (BeforeAll/AfterAll, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFuzzyRowFilterEndToEnd.java JUnit 5 migration (TestInfo, BeforeAll/AfterAll, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFuzzyRowFilter.java JUnit 5 migration (assertions, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFuzzyRowAndColumnRangeFilter.java JUnit 5 migration (BeforeAll/AfterAll, TestInfo, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterWrapper.java JUnit 5 migration (BeforeAll/AfterAll, tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterWithScanLimits.java JUnit 5 migration; explicit cluster setup via FilterTestingCluster.
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFiltersWithBinaryComponentComparator.java JUnit 5 migration (BeforeAll/AfterAll, TestInfo, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterSerialization.java JUnit 5 parameterized migration via @HBaseParameterizedTestTemplate and @TestTemplate.
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterListOrOperatorWithBlkCnt.java JUnit 5 migration (BeforeAll/AfterAll, TestInfo, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterListOnMini.java JUnit 5 migration (BeforeAll/AfterAll, TestInfo).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterList.java JUnit 5 migration (tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilterFromRegionSide.java JUnit 5 migration (BeforeAll/AfterAll, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java JUnit 5 migration (BeforeEach/AfterEach, TestInfo, Disabled).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestDependentColumnFilter.java JUnit 5 migration (BeforeEach/AfterEach, tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestComparatorSerialization.java JUnit 5 parameterized migration via @HBaseParameterizedTestTemplate and @TestTemplate.
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnRangeFilter.java JUnit 5 migration; moves helper class and updates lifecycle/tags.
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnPrefixFilter.java JUnit 5 migration (TestInfo, tags).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestColumnPaginationFilter.java JUnit 5 migration (BeforeEach, tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestBitComparator.java JUnit 5 migration (tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestBigDecimalComparator.java JUnit 5 migration (tags, assertions).
hbase-server/src/test/java/org/apache/hadoop/hbase/filter/FilterTestingCluster.java JUnit 5 migration; shifts cluster teardown to @AfterAll.
Comments suppressed due to low confidence (2)

hbase-server/src/test/java/org/apache/hadoop/hbase/tool/BulkLoadHFilesTestBase.java:378

  • String comparison uses != which compares references, not contents, so this cleanup assertion can behave incorrectly. Use !"DONOTERASE".equals(file.getPath().getName()) (or negate .equals) instead of !=.
    hbase-server/src/test/java/org/apache/hadoop/hbase/tool/SecureBulkLoadHFilesSplitRecoveryTest.java:66
  • This @Disabled method no longer overrides the inherited test because the base class testBulkLoadPhaseFailure now takes a TestInfo parameter. As a result, the inherited base test will still run in secure mode (and the intended disable won't take effect). Override with the same signature (testBulkLoadPhaseFailure(TestInfo)) and mark it @Disabled, or otherwise disable/exclude the base test for this subclass.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

results.clear();
}
assertEquals("Expected " + expectedRows + " rows but scanned " + i + " rows", expectedRows, i);
assertEquals(i, expectedRows, "Expected " + expectedRows + " rows but scanned " + i + " rows");
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertEquals argument order is reversed here (JUnit Jupiter is assertEquals(expected, actual, message)). This will report confusing failures and can hide issues if someone reads the output. Swap to assertEquals(expectedRows, i, ...).

Suggested change
assertEquals(i, expectedRows, "Expected " + expectedRows + " rows but scanned " + i + " rows");
assertEquals(expectedRows, i, "Expected " + expectedRows + " rows but scanned " + i + " rows");

Copilot uses AI. Check for mistakes.
Comment on lines +122 to +123
assertEquals(resultCount, 1);
assertEquals(cellCount, 4);
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertEquals argument order is reversed (JUnit Jupiter is assertEquals(expected, actual)). These assertions currently expect resultCount/cellCount to equal constants but pass them as the expected value, producing misleading failures. Swap to assertEquals(1, resultCount) and assertEquals(4, cellCount).

Suggested change
assertEquals(resultCount, 1);
assertEquals(cellCount, 4);
assertEquals(1, resultCount);
assertEquals(4, cellCount);

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@Apache9 Apache9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please try to run mvn test -PrunAllTests before and after applying the patch, to see if we have the same number of tests?

Seems we missed some tests in this PR, but since it is too big, I'm not sure I can figure out all the missing stuff...

}
}

@BeforeClass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No BeforeAll?

* By using this class as the super class of a set of tests you will have a HBase testing cluster
* available that is very suitable for writing tests for scanning and filtering against.
*/
@Category({ FilterTests.class, MediumTests.class })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing Tag?

@liuxiaocs7
Copy link
Member Author

Please try to run mvn test -PrunAllTests before and after applying the patch, to see if we have the same number of tests?

Seems we missed some tests in this PR, but since it is too big, I'm not sure I can figure out all the missing stuff...

Sorry for the mistake, let me double check!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants